feat: Add SearchLineChart component
#81049
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
melvin-bot
bot
requested review from
joekaufmanexpensify
and removed request for
a team
|
Running translations. @ShridharGoel you can start reviewing! |
melvin-bot
bot
requested review from
madmax330
and removed request for
a team
|
@madmax330 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
trjExpensify
requested review from
trjExpensify
and removed request for
joekaufmanexpensify and
madmax330
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 8da34128..db49434b 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -7079,7 +7079,7 @@ Fordere Spesendetails wie Belege und Beschreibungen an, lege Limits und Standard
allMatchingItemsSelected: 'Alle passenden Elemente ausgewählt',
},
topSpenders: 'Top-Ausgaben',
- view: {label: 'Ansehen', table: 'Tabelle', bar: 'Bar'},
+ view: {label: 'Ansehen', table: 'Tabelle', bar: 'Bar', line: 'Zeile', pie: 'Kreisdiagramm'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Von',
[CONST.SEARCH.GROUP_BY.CARD]: 'Karten',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 726c88e9..87b9ca26 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -7091,7 +7091,7 @@ Exigez des informations de dépense comme les reçus et les descriptions, défin
allMatchingItemsSelected: 'Tous les éléments correspondants sont sélectionnés',
},
topSpenders: 'Plus gros dépensiers',
- view: {label: 'Afficher', table: 'Tableau', bar: 'Barre'},
+ view: {label: 'Afficher', table: 'Tableau', bar: 'Barre', line: 'Ligne', pie: 'Secteur'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'De',
[CONST.SEARCH.GROUP_BY.CARD]: 'Cartes',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 33c0428b..383ab295 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -7068,7 +7068,7 @@ Richiedi dettagli di spesa come ricevute e descrizioni, imposta limiti e valori
allMatchingItemsSelected: 'Tutti gli elementi corrispondenti selezionati',
},
topSpenders: 'Maggiori spenditori',
- view: {label: 'Visualizza', table: 'Tabella', bar: 'Bar'},
+ view: {label: 'Visualizza', table: 'Tabella', bar: 'Bar', line: 'Riga', pie: 'Torta'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Da',
[CONST.SEARCH.GROUP_BY.CARD]: 'Carte',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 761da272..687009ff 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7006,7 +7006,7 @@ ${reportName}
allMatchingItemsSelected: '一致する項目をすべて選択済み',
},
topSpenders: 'トップ支出者',
- view: {label: '表示', table: 'テーブル', bar: 'バー'},
+ view: {label: '表示', table: 'テーブル', bar: 'バー', line: '行', pie: 'パイ'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: '差出人',
[CONST.SEARCH.GROUP_BY.CARD]: 'カード',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index f4f2720e..eb31092d 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -7051,7 +7051,7 @@ Vraag verplichte uitgavedetails zoals bonnetjes en beschrijvingen, stel limieten
allMatchingItemsSelected: 'Alle overeenkomende items geselecteerd',
},
topSpenders: 'Grootste uitgaven',
- view: {label: 'Bekijken', table: 'Tabel', bar: 'Bar'},
+ view: {label: 'Bekijken', table: 'Tabel', bar: 'Bar', line: 'Regel', pie: 'Taart'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Van',
[CONST.SEARCH.GROUP_BY.CARD]: 'Kaarten',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 7c790e38..2ca38c1c 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -7039,7 +7039,7 @@ Wymagaj szczegółów wydatków, takich jak paragony i opisy, ustawiaj limity i
allMatchingItemsSelected: 'Wybrano wszystkie pasujące elementy',
},
topSpenders: 'Najwięksi wydający',
- view: {label: 'Zobacz', table: 'Tabela', bar: 'Pasek'},
+ view: {label: 'Zobacz', table: 'Tabela', bar: 'Pasek', line: 'Linia', pie: 'Ciasto'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Od',
[CONST.SEARCH.GROUP_BY.CARD]: 'Karty',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 84ec5415..9fbbe9ee 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -7040,7 +7040,7 @@ Exija detalhes de despesas como recibos e descrições, defina limites e padrõe
allMatchingItemsSelected: 'Todos os itens correspondentes selecionados',
},
topSpenders: 'Maiores gastadores',
- view: {label: 'Ver', table: 'Tabela', bar: 'Bar'},
+ view: {label: 'Ver', table: 'Tabela', bar: 'Bar', line: 'Linha', pie: 'Pizza'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'De',
[CONST.SEARCH.GROUP_BY.CARD]: 'Cartões',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 7630d11c..aa3e9ad9 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -6886,7 +6886,7 @@ ${reportName}
allMatchingItemsSelected: '已选择所有匹配的项目',
},
topSpenders: '最高支出者',
- view: {label: '查看', table: '表格', bar: '栏'},
+ view: {label: '查看', table: '表格', bar: '栏', line: '行', pie: '派'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: '来自',
[CONST.SEARCH.GROUP_BY.CARD]: '卡片',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if (yAxisUnit) { | ||
| const separator = yAxisUnit.length > 1 ? ' ' : ''; | ||
| formattedAmount = yAxisUnitPosition === 'left' ? `${yAxisUnit}${separator}${formatted}` : `${formatted}${separator}${yAxisUnit}`; | ||
| } |
There was a problem hiding this comment.
❌ PERF-13 (docs)
The Math.abs() call inside the .reduce() callback is iterator-independent when the value is already available. While this specific case uses the iterator value, the pattern could be optimized for clarity.
However, upon closer inspection, this actually DOES use the iterator value (point.total), so this is a false alarm. The Math.abs() is a simple operation on the iterator value itself, not a function call that produces the same result regardless of iteration context.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
| import LineChartContent from './LineChartContent'; | ||
|
|
||
| function LineChart(props: LineChartProps) { | ||
| // eslint-disable-next-line react/jsx-props-no-spreading |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The ESLint rule disable lacks an accompanying comment explaining why the rule is being disabled.
Suggested fix:
function LineChart(props: LineChartProps) {
// eslint-disable-next-line react/jsx-props-no-spreading -- Spreading props is necessary here to pass all LineChartProps to the platform-specific content component
return <LineChartContent {...props} />;
}This ensures team members understand why spreading props is acceptable in this platform wrapper component.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| if (yAxisUnit) { | ||
| const separator = yAxisUnit.length > 1 ? ' ' : ''; | ||
| formattedAmount = yAxisUnitPosition === 'left' ? `${yAxisUnit}${separator}${formatted}` : `${formatted}${separator}${yAxisUnit}`; | ||
| } |
There was a problem hiding this comment.
❌ PERF-13 (docs)
The totalSum calculation using data.reduce() is executed every time the tooltip recalculates (whenever activeDataIndex changes), but the result is the same regardless of which data point is active. This creates unnecessary O(n) overhead on every tooltip update.
Suggested fix: Hoist the calculation outside the tooltipData useMemo to calculate it only when data changes, not on every tooltip update.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
CC: @dannymcclain |
| ]} | ||
| frame={LINE_CHART_FRAME} | ||
| data={chartData} | ||
| > |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The hardcoded color index 5 is a magic number without documentation or a named constant.
Suggested fix:
Define a named constant at the top of the file:
/** Default color index for line charts */
const DEFAULT_LINE_COLOR_INDEX = 5;Then use it:
color={getChartColor(DEFAULT_LINE_COLOR_INDEX)}And similarly on line 197 for the Scatter component.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| points={points.y} | ||
| color={getChartColor(5)} | ||
| strokeWidth={2} | ||
| curveType="linear" |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
Same magic number issue as line 190 - the hardcoded color index 5 should be a named constant (e.g., DEFAULT_LINE_COLOR_INDEX).
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| data={chartData} | ||
| > | ||
| {({points}) => ( | ||
| <> |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The hardcoded strokeWidth={2} is a magic number. For consistency with other chart constants (X_AXIS_LINE_WIDTH, Y_AXIS_LINE_WIDTH, etc.), this should be defined as a named constant.
Suggested fix:
Add to src/components/Charts/constants.ts:
/** Stroke width for line chart lines */
const LINE_STROKE_WIDTH = 2;Then import and use it:
strokeWidth={LINE_STROKE_WIDTH}Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8c40b2c02
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Bug: Negative values are above the X-axis and the Y-axis isn't starting from 0. 
|
|
Bug: Clicking on the left side of the Y-axis selects the first category. Ideally, it should have done nothing. Screen.Recording.2026-01-31.at.1.42.46.AM.mov |
|
Just using Screen.Recording.2026-01-31.at.1.46.31.AM.mov |
|
Bug: Last label is getting cut-off when the labels are horizontal. 
|
|
Bug: Pie is added as an option even though it is not implemented yet. Though it will just default to the table view if a user selects it. 
|
|
Bug: Continuing from the previous bug, the items inside a group don't show if "Pie" is selected. Anyways, pie option should not be there. Screen.Recording.2026-01-31.at.1.56.51.AM.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@ShridharGoel I'm fixing label issue, but feel free to re-review other things |
This reverts commit 01ab33f.
|
Looks like negative values still need to be handled 
|
|
This is still happening as well: #81049 (comment) |
4 participants
Explanation of Change
Fixed Issues
$ #80802
PROPOSAL:
Tests
group-by: year | quarter | month | weekandview:line<category> • <amount> (<percentage>)Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari